-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HMS-3141: image: add new ContainerBuildable
flag to OSTreeDiskImage
#287
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mvo5
added a commit
to mvo5/images
that referenced
this pull request
Nov 29, 2023
When trying to create enough data for a full pipeline run for osbuild#287 I struggled a bit because it was not clear which items where faulty. This commit adds more context to the errors. Sadly I had to implement `assertPanicsWithErrorRegexp` because of stretchr/testify#1304 I can contribute it upstream but for that it needs some tests around it first :)
mvo5
commented
Nov 29, 2023
cgwalters
reviewed
Nov 29, 2023
mvo5
added a commit
to mvo5/images
that referenced
this pull request
Nov 29, 2023
When trying to create enough data for a full pipeline run for osbuild#287 I struggled a bit because it was not clear which items where faulty. This commit adds more context to the errors. Sadly I had to implement `assertPanicsWithErrorRegexp` because of stretchr/testify#1304 I can contribute it upstream but for that it needs some tests around it first :)
mvo5
added a commit
to mvo5/images
that referenced
this pull request
Nov 29, 2023
When trying to create enough data for a full pipeline run for osbuild#287 I struggled a bit because it was not clear which items where faulty. This commit adds more context to the errors. Sadly I had to implement `assertPanicsWithErrorRegexp` because of stretchr/testify#1304 I can contribute it upstream but for that it needs some tests around it first :)
One objective for bifrost images is that it should be possible to run osbuild inside a container. This can interfere with the selinux policies of the buildroot. Inside the container everything is labeled `system_u:object_r:container_files_t`. Labeling /usr/bin/osbuild as `osbuild_exec_t` is not possible in the general case because the host may not have `osbuild-selinux` installed that contains this type. The workaround is that the container labels osbuild itself as `install_exec_t`. This works fine however there is a selinux denial warning when the `{,u}mount` binary is called because the transition from `install_t`->`mount_t` is not allowed. The warning is "harmless" because `install_t` has enough privs to allow the `{,u}mount` binaries to work. To silence this warning we can label `{,u}mount` in the buildroot as `install_exec_t` directly. This commit allows to control this now via the `ContainerBuildable` flag that can be set on `manifest.Build` to enable this behavior.
mvo5
force-pushed
the
bifrost-image-no-denials
branch
from
November 29, 2023 16:32
f007dbe
to
929f49b
Compare
mvo5
added a commit
to mvo5/images
that referenced
this pull request
Dec 4, 2023
When trying to create enough data for a full pipeline run for osbuild#287 I struggled a bit because it was not clear which items where faulty. This commit adds more context to the errors. It uses the new `assertx.PanicsWithErrorRegexp` that is needed because of stretchr/testify#1304 (until fixed or contributed back).
4 tasks
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 4, 2023
When trying to create enough data for a full pipeline run for #287 I struggled a bit because it was not clear which items where faulty. This commit adds more context to the errors. It uses the new `assertx.PanicsWithErrorRegexp` that is needed because of stretchr/testify#1304 (until fixed or contributed back).
mvo5
changed the title
image: add new
image: add new Dec 5, 2023
ContainerBuildable
flag to OSTreeDiskImageContainerBuildable
flag to OSTreeDiskImage (HMS-3141)
achilleas-k
approved these changes
Dec 7, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. Should we mark it ready or does it need more work?
Thank you! I marked it ready, I only left it in draft as I wanted to make sure the API is okay :) |
mvo5
added a commit
to mvo5/bootc-image-builder
that referenced
this pull request
Dec 7, 2023
With the new `image.NewOSTreeDiskImageFromContainer().ContainerBuildable` option it is now possible to set the selinux policy so that there are no selinux denials even when build in a container without osbuild-selinux on the host. See osbuild/images#287 for details.
ochosi
changed the title
image: add new
HMS-3141: image: add new Dec 7, 2023
ContainerBuildable
flag to OSTreeDiskImage (HMS-3141)ContainerBuildable
flag to OSTreeDiskImage
github-merge-queue bot
pushed a commit
to osbuild/bootc-image-builder
that referenced
this pull request
Dec 7, 2023
With the new `image.NewOSTreeDiskImageFromContainer().ContainerBuildable` option it is now possible to set the selinux policy so that there are no selinux denials even when build in a container without osbuild-selinux on the host. See osbuild/images#287 for details.
mvo5
added a commit
to mvo5/images
that referenced
this pull request
Jan 25, 2024
Similar as osbuild#287 we need to label `/usr/bin/{mount,umount}` as `install_exec_t` to prevent an selinux denial warning when osbuild runs mount/unmount. See dea1af4 for more details.
mvo5
added a commit
to mvo5/images
that referenced
this pull request
Jan 25, 2024
Similar as osbuild#287 we need to label `/usr/bin/{mount,umount}` as `install_exec_t` to prevent an selinux denial warning when osbuild runs mount/unmount. See dea1af4 for more details.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
One objective for bifrost images is that it should be possible to run osbuild inside a container. This can interfere with the selinux policies of the buildroot.
Inside the container everything is labeled
system_u:object_r:container_files_t
. Labeling /usr/bin/osbuild asosbuild_exec_t
is not possible in the general case because the host may not haveosbuild-selinux
installed that contains this type. The workaround is that the container labels osbuild itself asinstall_exec_t
. This works fine however there is a selinux denial warning when the{,u}mount
binary is called because the transition frominstall_t
->mount_t
is not allowed. The warning is "harmless" becauseinstall_t
has enough privs to allow the{,u}mount
binaries to work.To silence this warning we can label
{,u}mount
in the buildroot asinstall_exec_t
directly.This commit allows to control this now via the
ContainerBuildable
flag that can be set onmanifest.Build
to enable this behavior.Once this has landed https://github.com/osbuild/osbuild-deploy-container/pull/11/files#diff-895717f8731ac528a1e229abd5c4b2d22eb2c471412ea30c1df61b88dd90e4f5R49 will need an update to do
img.ContainerBuildlable = true